BLD: Add CMake build using scikit-build-core #242

DWesl · 2024-06-13T15:27:14Z

Builds on pyproject.toml added in #241.

I think I got every key from setup.py. The main difference is if the READTHEDOCS environment variable is set: I don't know how to change the dependencies based on that. On the other hand, given 3.8 is close to or past EOL, I suspect 3.3 is relatively uncommon, and the RTD install, without the unneeded bits, could be accomplished with pip install --no-deps.

…tes. numpy.distutils recommends setuptools<60, but that breaks pyproject.toml configuration. I like this way better. Still needs setup.py for the fortran module and for the Cheyenne configuration.

setuptools<60 just gets confused, and has no idea what the project name is. I just inverted that requirement: not sure if setuptools needs to be more recent still.

Works decently as a standalone builder/installer. Need to work on packaging for python. Scikit-build-core says it's a good option.

I should delete setup.py before too much longer, shouldn't I.

Pretty sure none of them are designed to be used that way. Also it kept CMake from trying to execute them with sh.

Can't test locally. Debating testing in my repo or draft PR.

It's not working, and I'd like to know why.

Only use my versions when skbuild isn't around.

It's not working, and it doesn't look easy to get working on Mac. Everything should be working now, so let's check.

Requirements list on RTD is much shorter than elsewhere, so do not install the extra packages on RTD. The last part of setup.py left is the Cheyenne configuration.

kafitzgerald · 2024-06-13T16:09:35Z

Thanks so much for your work here! I just noticed this and your prior PR.

Admittedly, the maintenance on this package has been a struggle and difficult to prioritize.

I'll try to take a look at these shortly.

kafitzgerald · 2024-07-23T15:43:33Z

I started looking into this, but haven't been able to get it to build without error. I'm seeing something similar to what's showing up in CI.

I'll try to take a deeper look soon as moving away from distutils is something I'd like to support / make happen.

For some reason this isn't getting written despite every file being .f90 and no file being .f77, so put that in the code.

Update package metadata.

Let's see if this allows this to build. I haven't had problems locally with 3.9/1.26

Plantain · 2024-09-13T00:46:04Z

This builds for me, but it fails to actually import at runtime for unclear reasons:

ImportError: /usr/local/lib/python3.12/dist-packages/wrf/_wrffortran.cpython-312-x86_64-linux-gnu.so: undefined symbol: f2pyinitomp_constants_

DWesl · 2024-09-13T12:24:52Z

This builds for me, but it fails to actually import at runtime for unclear reasons:

ImportError: /usr/local/lib/python3.12/dist-packages/wrf/_wrffortran.cpython-312-x86_64-linux-gnu.so: undefined symbol: f2pyinitomp_constants_

That's a new one. Usually it's complaining about the OpenMP symbols directly. Progress?

On a related note, I think I should look into how to enable OpenMP, since I don't remember doing that.

Plantain · 2024-09-13T23:48:48Z

I made a reproduction example in a Docker container.

Dockerfile:

FROM debian:sid
RUN apt-get update && apt-get -y upgrade && apt-get -y dist-upgrade && apt-get -y install curl gfortran build-essential libssl-dev zlib1g-dev \
libbz2-dev libreadline-dev libsqlite3-dev curl git cmake \
libncursesw5-dev xz-utils tk-dev libxml2-dev libxmlsec1-dev libffi-dev liblzma-dev
ENV PYENV_ROOT /root/.pyenv
ENV PATH $PYENV_ROOT/shims:$PYENV_ROOT/bin:$PATH
RUN curl https://pyenv.run | bash && pyenv install 3.12 && pyenv global 3.12
RUN python3 -m pip install --upgrade setuptools numpy
RUN python3 -m pip install git+https://github.com/DWesl/wrf-python.git@cmake-build

root:~/docker/wrftest# docker build . -t test
[+] Building 0.6s (9/9) FINISHED                                                                                                                                                                                                   docker:default
 => [internal] load build definition from Dockerfile                                                                                                                                                                                         0.0s
 => => transferring dockerfile: 632B                                                                                                                                                                                                         0.0s
 => [internal] load metadata for docker.io/library/debian:sid                                                                                                                                                                                0.6s
 => [internal] load .dockerignore                                                                                                                                                                                                            0.0s
 => => transferring context: 2B                                                                                                                                                                                                              0.0s
 => [1/5] FROM docker.io/library/debian:sid@sha256:08cc1a90963e48072614d24e816bb659e62ca9db485151785e12331998766627                                                                                                                          0.0s
 => CACHED [2/5] RUN apt-get update && apt-get -y upgrade && apt-get -y dist-upgrade && apt-get -y install curl gfortran build-essential libssl-dev zlib1g-dev libbz2-dev libreadline-dev libsqlite3-dev curl git cmake libncursesw5-dev xz  0.0s
 => CACHED [3/5] RUN curl https://pyenv.run | bash && pyenv install 3.12 && pyenv global 3.12                                                                                                                                                0.0s
 => CACHED [4/5] RUN python3 -m pip install --upgrade setuptools numpy                                                                                                                                                                       0.0s
 => CACHED [5/5] RUN python3 -m pip install git+https://github.com/DWesl/wrf-python.git@cmake-build                                                                                                                                          0.0s
 => exporting to image                                                                                                                                                                                                                       0.0s
 => => exporting layers                                                                                                                                                                                                                      0.0s
 => => writing image sha256:953d34e9016f91f43dc86f06f2b0818a18c8e46c1d4550eda2f112066cca735d                                                                                                                                                 0.0s
 => => naming to docker.io/library/test                                                                                                                                                                                                      0.0s
root:~/docker/wrftest# docker run -ti test
root@181102790b9a:/# python3
Python 3.12.6 (main, Sep 13 2024, 23:33:23) [GCC 14.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from wrf import getvar
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/root/.pyenv/versions/3.12.6/lib/python3.12/site-packages/wrf/__init__.py", line 6, in <module>
    from . import api
  File "/root/.pyenv/versions/3.12.6/lib/python3.12/site-packages/wrf/api.py", line 1, in <module>
    from .config import (xarray_enabled, disable_xarray, enable_xarray,
  File "/root/.pyenv/versions/3.12.6/lib/python3.12/site-packages/wrf/config.py", line 6, in <module>
    from ._wrffortran import (fomp_enabled, fomp_set_num_threads,
ImportError: cannot import name 'omp_constants' from 'wrf._wrffortran' (/root/.pyenv/versions/3.12.6/lib/python3.12/site-packages/wrf/_wrffortran.cpython-312-x86_64-linux-gnu.so)
>>>

I think I had it earlier, but deleted it from the link line because I thought the library was fixed-form only, rather than making sure it was available for the link line.

DWesl · 2024-09-16T21:46:30Z

It appears that numpy.f2py is not generating the .f90 file that would produce a definition for module omp_constants because that module is used and has no functions, only variables. I do not see an obvious manner to override this choice.

Let's see if that's the fix that broke the build

akrherz · 2024-09-17T00:15:59Z

It appears that numpy.f2py is not generating the .f90 file that would produce a definition for module omp_constants because that module is used and has no functions, only variables. I do not see an obvious manner to override this choice.

I think that issue is described here numpy/numpy#19161 , I tried adding a dummy function to trick this, but my fortran foo was not strong enough

f2py seems to do better with those.

Mac doesn't call it ldd, not sure what it should be.

DWesl

Not sure if these should be matched to the versions in CI as well, but it wouldn't hurt.

CMakeLists.txt

pyproject.toml

kafitzgerald · 2024-10-10T00:55:14Z

🎉 this is building for me locally now and passing tests.

It looks like there are a few merge conflicts after the last PR was merged. @DWesl is this something you'd like to address or should I go ahead and do it?

I'd like to take a bit more in depth look at the PR itself as well, but hopefully we can get this merged here soon.

Thanks again for your work here!

DWesl · 2024-10-15T00:25:45Z

I haven't managed to test on Python 3.7, so I'm not sure if I should keep the Python>=3.7 requirement from pyproject.toml or the Python>=3.8 requirement in CMakeLists.txt. Either way, the versions should match.

Nor, for that matter, have I managed to test on Python 3.8, I think due to a different issue.

kafitzgerald

Just a few suggestions here, mostly for consistency.

CMakeLists.txt

pyproject.toml

kafitzgerald · 2024-10-23T00:06:58Z

CMakeLists.txt

+# TODO: Figure out the conditionals for running the C Preprocessor on Fortran files
+# I think the main thing to be changed is -E -cpp
+# Intel is -fpp -save-temps or /fpp on Windows
+# or call fpp instead of the fortran compiler to get it to stop after preprocessing


It'd be good to look into this for Windows at least, but I think that can happen after getting this merged. Maybe along with some testing for Windows.

Don't specify Python version in CMakeLists.txt Co-authored-by: Katelyn FitzGerald <[email protected]>

DWesl added 18 commits May 23, 2024 16:10

BLD,BUG: Fix pyproject.toml configuration and remove setup.py duplica…

8322c42

…tes. numpy.distutils recommends setuptools<60, but that breaks pyproject.toml configuration. I like this way better. Still needs setup.py for the fortran module and for the Cheyenne configuration.

FIX: Require a setuptools version that understands the configuration.

5c169eb

setuptools<60 just gets confused, and has no idea what the project name is. I just inverted that requirement: not sure if setuptools needs to be more recent still.

BLD: Start working on a CMake build.

ca51e70

Works decently as a standalone builder/installer. Need to work on packaging for python. Scikit-build-core says it's a good option.

BLD: Initial Scikit-build attempt.

e88a5bd

I should delete setup.py before too much longer, shouldn't I.

STY: Remove execute permissions from python modules.

d18ecf5

Pretty sure none of them are designed to be used that way. Also it kept CMake from trying to execute them with sh.

BLD: First attempt at OpenMP.

4686af4

Can't test locally. Debating testing in my repo or draft PR.

CI: Update CI for CMake build, and add 3.12

ad35d87

CI: Check the installed package.

937e164

It's not working, and I'd like to know why.

CI: Fix Python 3.12 install and package install check

ddcf9eb

FIX: Try to get checks working.

05657f8

BLD: Let skbuild handle install dirs when available.

0df03d8

Only use my versions when skbuild isn't around.

FIX: Fix syntax in CMake variable reference.

0d38af9

CI,DBG: Only run ldd on so files.

a962d9f

FIX: Ensure grep knows the patterns are fixed strings.

028ec66

CI,DBG: Remove debug section.

3b9abc9

It's not working, and it doesn't look easy to get working on Mac. Everything should be working now, so let's check.

FIX: Fix install directory for fortran extension module.

8944514

BLD: Move requirements.txt to pyproject.toml

dd8ead2

Requirements list on RTD is much shorter than elsewhere, so do not install the extra packages on RTD. The last part of setup.py left is the Cheyenne configuration.

DWesl mentioned this pull request Jun 13, 2024

error when install wrf-python #237

Closed

DWesl added 6 commits August 20, 2024 13:20

BLD: Do not expect f2py to produce ${modname}-f2pywrappers2.f90

b3557c3

For some reason this isn't getting written despite every file being .f90 and no file being .f77, so put that in the code.

DOC: Note that this should work on Python 3.12

0038273

Update package metadata.

Merge branch 'develop' of NCAR/wrf-python into cmake-build

b940960

Merge branch 'cmake-build' of DWesl/wrf-python into cmake-build

e1f530e

Merge branch 'develop' into cmake-build

51cea52

BLD: Pin NumPy<2

9654ada

Let's see if this allows this to build. I haven't had problems locally with 3.9/1.26

BLD: Make CMake OpenMP linkage public.

7944265

DWesl added 3 commits September 15, 2024 15:57

BUG: Fix spelling of Python_EXECUTABLE in CMakeLists.txt

e41f75e

BUG: Include the f90 wrapper file in the compilation.

379691d

I think I had it earlier, but deleted it from the link line because I thought the library was fixed-form only, rather than making sure it was available for the link line.

FIX: Fix syntax in CMakeLists.txt

b5fdd5f

BLD: Pin numpy<1.26.3

4bde3cf

Let's see if that's the fix that broke the build

DWesl added 4 commits September 17, 2024 10:03

ENH: Add a function to module wrf_constants so f2py includes it

0eed892

ENH: Add a function to module omp_constants so f2py includes it

607cf30

FIX: Fix where in module functions go

1c20f4a

BUG: Fix where in module functions go

ef93ca6

akrherz added a commit to regro-cf-autotick-bot/wrf-python-feedstock that referenced this pull request Sep 17, 2024

try patch from NCAR/wrf-python#242

c639902

akrherz added a commit to regro-cf-autotick-bot/wrf-python-feedstock that referenced this pull request Sep 17, 2024

try patch from NCAR/wrf-python#242

dbb3956

DWesl added 8 commits September 17, 2024 10:50

ENH: Make have_wrf_constants a subroutine

f914a85

f2py seems to do better with those.

BLD: Remove pin on NumPy version

46cf6b3

CI,DBG: Re-add debug section to CI

9b712c8

CI,BUG: Try to fix debug section.

faeda9d

CI,BUG: Try to get ls ${installed_files} working.

bb4ff8e

CI,BUG: Don't run ldd until after python import

80265d6

Mac doesn't call it ldd, not sure what it should be.

BLD,BUG: Try a different suffix for extension modules

c9573de

CI,ENH: conda package build renamed to python-build

f9f690f

DWesl commented Sep 18, 2024

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

pyproject.toml Outdated Show resolved Hide resolved

Merge branch 'develop' of NCAR/wrf-python into cmake-build

e502213

kafitzgerald reviewed Oct 23, 2024

View reviewed changes

BLD: Update python_version to >=3.9 in pyproject.toml

97643e7

Don't specify Python version in CMakeLists.txt Co-authored-by: Katelyn FitzGerald <[email protected]>

kafitzgerald approved these changes Oct 25, 2024

View reviewed changes

kafitzgerald merged commit 69fd6a4 into NCAR:develop Oct 25, 2024
13 checks passed

DWesl deleted the cmake-build branch October 25, 2024 13:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BLD: Add CMake build using scikit-build-core #242

BLD: Add CMake build using scikit-build-core #242

DWesl commented Jun 13, 2024

kafitzgerald commented Jun 13, 2024

kafitzgerald commented Jul 23, 2024

Plantain commented Sep 13, 2024 •

edited

Loading

DWesl commented Sep 13, 2024

Plantain commented Sep 13, 2024

DWesl commented Sep 16, 2024

akrherz commented Sep 17, 2024

DWesl left a comment

kafitzgerald commented Oct 10, 2024

DWesl commented Oct 15, 2024 •

edited

Loading

kafitzgerald left a comment

kafitzgerald Oct 23, 2024

BLD: Add CMake build using scikit-build-core #242

BLD: Add CMake build using scikit-build-core #242

Conversation

DWesl commented Jun 13, 2024

kafitzgerald commented Jun 13, 2024

kafitzgerald commented Jul 23, 2024

Plantain commented Sep 13, 2024 • edited Loading

DWesl commented Sep 13, 2024

Plantain commented Sep 13, 2024

DWesl commented Sep 16, 2024

akrherz commented Sep 17, 2024

DWesl left a comment

Choose a reason for hiding this comment

kafitzgerald commented Oct 10, 2024

DWesl commented Oct 15, 2024 • edited Loading

kafitzgerald left a comment

Choose a reason for hiding this comment

kafitzgerald Oct 23, 2024

Choose a reason for hiding this comment

Plantain commented Sep 13, 2024 •

edited

Loading

DWesl commented Oct 15, 2024 •

edited

Loading